chore(ibis): add v3 metadata API and remove v2 Oracle test#1369
Closed
goldmedal wants to merge 2 commits intoCanner:mainfrom
Closed
chore(ibis): add v3 metadata API and remove v2 Oracle test#1369goldmedal wants to merge 2 commits intoCanner:mainfrom
goldmedal wants to merge 2 commits intoCanner:mainfrom
Conversation
Contributor
WalkthroughThree new v3 metadata endpoints were added to the connector router for retrieving table lists, database constraints, and database version. The Oracle v2 connector test suite file was removed and three v3 Oracle metadata tests were added. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant V3Endpoint as V3 Metadata Endpoint
participant Factory as MetadataFactory
participant Executor as Timeout Executor
participant DB as Database
Client->>V3Endpoint: POST /{data_source}/metadata/tables (connectionInfo, dto)
activate V3Endpoint
V3Endpoint->>V3Endpoint: start tracing span\nattach headers
V3Endpoint->>Factory: request Metadata instance
activate Factory
Factory->>DB: connect / initialize
Factory-->>V3Endpoint: Metadata instance
deactivate Factory
V3Endpoint->>Executor: execute_get_table_list_with_timeout()
activate Executor
Executor->>DB: fetch table metadata
Executor-->>V3Endpoint: return list[Table]
deactivate Executor
V3Endpoint-->>Client: 200 OK + payload
deactivate V3Endpoint
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ibis-server/app/routers/v3/connector.py (1)
450-507: LGTM! Metadata endpoints properly implemented.The three new v3 metadata endpoints follow the established patterns and are consistent with their v2 counterparts. The implementation correctly:
- Uses appropriate FastAPI decorators and response models
- Handles headers and connection info properly
- Leverages
MetadataFactoryand timeout wrappers- Creates tracing spans for observability
Optional: Add tracing span to
get_db_versionfor consistency.The
get_db_versionendpoint (lines 494-507) doesn't create a tracing span, whileget_table_listandget_constraintsdo. For consistency and better observability, consider adding a span:async def get_db_version( data_source: DataSource, dto: MetadataDTO, headers: Annotated[Headers, Depends(get_wren_headers)] = None, ) -> str: + span_name = f"v3_metadata_version_{data_source}" + with tracer.start_as_current_span( + name=span_name, kind=trace.SpanKind.SERVER, context=build_context(headers) + ) as span: + set_attribute(headers, span) - connection_info = data_source.get_connection_info( - dto.connection_info, dict(headers) - ) - metadata = MetadataFactory.get_metadata(data_source, connection_info) - return await execute_get_version_with_timeout(metadata) + connection_info = data_source.get_connection_info( + dto.connection_info, dict(headers) + ) + metadata = MetadataFactory.get_metadata(data_source, connection_info) + return await execute_get_version_with_timeout(metadata)Note: This matches the current v2 behavior, so the inconsistency exists in both versions. This is purely an optional enhancement for uniformity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
ibis-server/app/routers/v3/connector.py(3 hunks)ibis-server/tests/routers/v2/connector/test_oracle.py(0 hunks)ibis-server/tests/routers/v3/connector/oracle/test_query.py(1 hunks)
💤 Files with no reviewable changes (1)
- ibis-server/tests/routers/v2/connector/test_oracle.py
🧰 Additional context used
🧬 Code graph analysis (2)
ibis-server/tests/routers/v3/connector/oracle/test_query.py (7)
ibis-server/tests/routers/v2/connector/test_local_file.py (4)
test_metadata_list_tables(186-213)connection_info(85-89)test_metadata_list_constraints(216-223)test_metadata_db_version(226-234)ibis-server/tests/routers/v2/connector/test_trino.py (3)
test_metadata_list_tables(274-319)test_metadata_list_constraints(322-331)test_metadata_db_version(334-341)ibis-server/tests/routers/v2/connector/test_mysql.py (3)
test_metadata_list_tables(279-308)test_metadata_list_constraints(311-325)test_metadata_db_version(328-335)ibis-server/tests/routers/v2/connector/test_snowflake.py (2)
test_metadata_list_tables(273-300)test_metadata_list_constraints(303-311)ibis-server/tests/routers/v2/connector/test_postgres.py (3)
test_metadata_list_tables(689-715)test_metadata_list_constraints(718-724)test_metadata_db_version(727-734)ibis-server/tests/conftest.py (1)
client(18-23)ibis-server/tests/routers/v3/connector/oracle/conftest.py (1)
connection_info(102-111)
ibis-server/app/routers/v3/connector.py (5)
ibis-server/app/model/metadata/dto.py (3)
Constraint(94-100)MetadataDTO(9-10)Table(80-85)ibis-server/app/model/metadata/factory.py (2)
MetadataFactory(43-62)get_metadata(45-62)ibis-server/app/util.py (5)
execute_get_constraints_with_timeout(361-368)execute_get_table_list_with_timeout(351-358)execute_get_version_with_timeout(371-378)build_context(142-145)set_attribute(148-155)ibis-server/app/routers/v2/connector.py (3)
get_table_list(259-273)get_constraints(282-296)get_db_version(304-313)ibis-server/app/model/data_source.py (2)
DataSource(60-214)get_connection_info(90-135)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci
🔇 Additional comments (3)
ibis-server/tests/routers/v3/connector/oracle/test_query.py (2)
263-290: LGTM! Comprehensive metadata table test.The test properly validates the metadata response structure including table properties, columns, and specific column details. The assertions align with the expected Oracle metadata format and match patterns used in other datasource tests.
304-310: LGTM! Database version test is straightforward.The test correctly validates that the Oracle database version endpoint returns a 200 status and contains the expected version string "23.0".
ibis-server/app/routers/v3/connector.py (1)
34-35: LGTM! Appropriate imports for metadata endpoints.The new imports for metadata types (
Constraint,MetadataDTO,Table,MetadataFactory) and timeout execution helpers are correctly added to support the new v3 metadata endpoints.Also applies to: 44-46
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Tests